-
-
Notifications
You must be signed in to change notification settings - Fork 15.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move java classes out of the native modules #11798
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the way to go.
You also need to tell japicmp that this is fine, though. |
Motivation: To ensure the user doesnt end up with the same classes multiple times in the classpath when including native dependencies for different platforms we need to ensure we not include the java classes in this jars but provide them as seperate modules. Modifications: - Introduces a *-classes-* module for each *-native-* module and depend on it from the native module - Add entries to the bom - Depend on the correct artifacts in netty-all to ensure we not end up with the same classes multiple times Result: Fixes #11791
59422d0
to
29f8bcc
Compare
--> | ||
<dependency> | ||
<groupId>${project.groupId}</groupId> | ||
<artifactId>netty-transport-native-epoll</artifactId> | ||
<artifactId>netty-transport-classes-epoll</artifactId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if the name classes
makes sense although I don't have a good idea on this. Maybe common
? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I picked classes as it contains the classes but no idea 🤷♂️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or just nothing? netty-transport-epoll
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forgot to add don't
to have a good idea
😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either nothing or classes
seem fine to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about it, I think I have a slight preference for keeping it. Without, it might be the first dependency people add, only to discover it doesn’t work because the native binaries are missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about it, I think I have a slight preference for keeping it. Without, it might be the first dependency people add, only to discover it doesn’t work because the native binaries are missing.
@trustin works for you ?
Let me merge this one so we can make progress. |
@normanmaurer Is it intended to have netty-transport-native-unix-common-4.1.70.Final.jar with class files? Or are they different from the ones you moved here? (jar size of 4.1.69 similar) |
yes this is not a problem |
netty-bom 4.1.70 contains the changes from pull request netty/netty#11798, which moved the classes out of the native modules to make sure the same classes don't end up on the classpath multiple times. For us it should mean that we don't have to depend on the specific native module any more but can simply depend on the common/shared classes module.
netty-bom 4.1.70 contains the changes from pull request netty/netty#11798, which moved the classes out of the native modules to make sure the same classes don't end up on the classpath multiple times. For us it means that we need to depend on both the native and classes modules. However, since we don't use the native module directly (only classes that were moved to this classes module), we need to force the dependency plugin to consider the native module as used.
Motivation: To ensure the user doesnt end up with the same classes multiple times in the classpath when including native dependencies for different platforms we need to ensure we not include the java classes in this jars but provide them as seperate modules. Modifications: - Introduces a *-classes-* module for each *-native-* module and depend on it from the native module - Add entries to the bom - Depend on the correct artifacts in netty-all to ensure we not end up with the same classes multiple times Result: Fixes netty#11791
Motivation: To ensure the user doesnt end up with the same classes multiple times in the classpath when including native dependencies for different platforms we need to ensure we not include the java classes in this jars but provide them as seperate modules. Modifications: - Introduces a *-classes-* module for each *-native-* module and depend on it from the native module - Add entries to the bom - Depend on the correct artifacts in netty-all to ensure we not end up with the same classes multiple times Result: Fixes netty#11791
netty-bom 4.1.70 contains the changes from pull request netty/netty#11798, which moved the classes out of the native modules to make sure the same classes don't end up on the classpath multiple times. For us it means that we need to depend on both the native and classes modules. However, since we don't use the native module directly (only classes that were moved to this classes module), we need to force the dependency plugin to consider the native module as used.
Motivation: To ensure the user doesnt end up with the same classes multiple times in the classpath when including native dependencies for different platforms we need to ensure we not include the java classes in this jars but provide them as seperate modules. Modifications: - Introduces a *-classes-* module for each *-native-* module and depend on it from the native module - Add entries to the bom - Depend on the correct artifacts in netty-all to ensure we not end up with the same classes multiple times Result: Fixes netty#11791
Motivation:
To ensure the user doesnt end up with the same classes multiple times in the classpath when including native dependencies for different platforms we need to ensure we not include the java classes in this jars but provide them as separate modules.
Modifications:
Result:
Fixes #11791